fix(supply-chain): SC4 must not claim a vulnerability it did not verify (#318) - #319
Open
Mark2Mac wants to merge 4 commits into
Open
fix(supply-chain): SC4 must not claim a vulnerability it did not verify (#318)#319Mark2Mac wants to merge 4 commits into
Mark2Mac wants to merge 4 commits into
Conversation
….txt
`_extract_packages_from_requirements` kept the captured version for any operator,
so a floor like `pillow>=10.0.0` was recorded as the exact release `10.0.0` and
the OSV/CVE lookup attributed that version's vulnerabilities to an unpinned
dependency — a false CRITICAL on a requirements file that pins nothing.
Only `==` and `<=` bound the dependency to a concrete, CVE-checkable release;
`>=`, `>`, `!=`, `~=` are floors/ranges. This mirrors the guard already present
in `_extract_packages_from_setup_py` (`m.group(2) in ("==", "<=")`), so the two
extractors now agree.
Added a regression test asserting `>=`, `~=`, `!=` and bare names yield
version=None while `==` / `<=` keep the version.
Fixes NVIDIA#294
Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com>
Addresses the review on NVIDIA#302: the previous guard still admitted non-exact constraints. `<=8.1.0` matches every earlier release and `==1.*` is a wildcard, so both were handed to the vulnerability lookup as a version the dependency may never install. A vulnerability lookup answers "is THIS release affected?", which is only meaningful when the manifest admits exactly one release. That predicate is now explicit and shared instead of being re-derived at each call site: - `_pinned_version` (PEP 440): only `==` with a fully concrete version. Floors, caps, exclusions, compatible releases and wildcard equality yield None. - `_pinned_npm_version` (semver): only a bare `x.y.z`. npm defaults to caret ranges, so `"^1.8.3"` was being stripped into the concrete release `1.8.3`. Applied to all three extractors — requirements.txt, pyproject.toml and package.json — because the objection in the review holds verbatim for the two that were not touched by the original patch. Note for the maintainer: dropping these specifiers moves more dependencies to version=None, which NVIDIA#318 shows is currently reported as CRITICAL carrying the package's worst-ever advisory. The two fixes are complementary; happy to send the severity side as a separate PR. Regressions cover both cases named in the review (`<=` and `==1.*`) plus the npm caret/tilde/wildcard/range forms. Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com>
Contributor
Author
|
Cross-PR note, from merging all my open PRs together locally. This PR and #302 interact, and it is worth seeing before either lands. Real example from the corpus: Today the In other words the false pin was accidentally producing silence in this case, and the two PRs together turn it into an honest low-severity note rather than either a wrong CRITICAL or nothing at all. Neither ordering is broken; both applied, the full suite is green ( |
rng1995
previously approved these changes
Jul 31, 2026
rng1995
left a comment
rng1995
left a comment
Collaborator
There was a problem hiding this comment.
[Automated SkillSpector Review]
Approved. A name-only OSV query is now reported as unverifiable rather than as a version-matched vulnerability: severity is capped at LOW, confidence is reduced, and the message/matched text no longer invent an exact version. Exact-version matches preserve the OSV severity and wording. The focused supply-chain suite passes (242 tests).
When a manifest admits a range, no version is resolved and OSV is queried by name alone. The advisories that come back are the package's history, not a match against the release that will be installed: the worst of them may predate every version the range admits. Using that as the finding's severity turns 'setuptools>=61' into a CRITICAL 'Known Vulnerable Dependency'. Scanning 65 skill/plugin units, every SC4 finding in the corpus came from this or from a range being read as a pin (NVIDIA#294/NVIDIA#302). Not one manifest pinned a vulnerable release. The lack of pinning is already reported by SC1, so what is left for SC4 to say is 'could not verify', and it must not outrank a real version match: severity capped at LOW, confidence 0.4, and wording that states the limit instead of implying a match. Version-matched findings are unchanged. Closes NVIDIA#318 Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com>
keshprad
force-pushed
the
fix/sc4-unresolved-version-severity
branch
from
August 4, 2026 03:11
b85d20d to
4705eeb
Compare
Signed-off-by: keshavp <32313895+keshprad@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #318.
The problem
When a manifest admits a range, no version is resolved and
query_batchis called with(name, None). OSV then answers a different question than the one SC4 asks: not "is this release affected?" but "has this package ever had an advisory?"._sc4_from_osvtakes the worst severity of that history and reports it as the finding's severity, so:The floor excludes every affected release. Nothing here was verified.
Why the severity, specifically
The lack of pinning is already reported by SC1 ("Dependencies lack version pinning"), so SC4 is not the only signal and nothing is lost by making it honest. What is left for SC4 to say in this case is "could not verify" — and that must not outrank a finding where a version genuinely matched a vulnerable release.
So: severity capped at
LOW, confidence0.4, and wording that states the limit (Unverifiable Dependency: … the manifest does not pin a version, so it is unknown whether the installed release is affected) instead of implying a match.matched_textno longer carries a==that the manifest never had. Version-matched findings are completely unchanged.Field data
Scanning 65 skill/plugin units: every SC4 finding in the corpus came from this behaviour or from a range being read as a pin (#294 / #302). Not one manifest pinned a vulnerable release. Two of the affected files are first-party marketplaces, so the practical effect today is that a user either fixes a non-problem or learns to ignore SC4 entirely.
Relationship to #302
Complementary halves of the same defect, and neither is sufficient alone — measured on the same inputs:
setuptools>=61"shell-quote": "^1.8.3"==1.8.3==1.8.3jinja2==2.4.1(really vulnerable)#302 stops inventing a version; this PR stops claiming a match that was never made.
Tests
Three regressions on
_sc4_from_osv: pinned version keeps the OSV severity and thename==versionwording; unresolved version is capped, de-confidenced and reworded; no advisories still emits nothing. Full unit suite green (696 passed, 14 skipped).